docs(ref): Clarify MSRV is generally minor #12122
Conversation
For the second time in the last week or two, I've seen the wording of cargo's semver documentation used to justify pushing back against treating MSRV bumps as minor changes. See time-rs/time#535 (comment) The current wording makes it sound like "major" is the default stance but the [general consensus seems to be around "minor"](rust-lang/api-guidelines#231). I held back from changing from "possibly major" to minor" for now as the above linked policy isn't official yet and it leaves it open for crates to treat it as major.
|
r? @ehuss (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
A-documenting-cargo-itself
|
Yahooo! @epage thanks for doing this!!! Totally understand that this documentation is meant to be a guideline, and in no way is the official rule for MSRV, but I think updating the docs here will help many others like me! Thanks again!! |
| @@ -1251,7 +1251,8 @@ projects that are using older versions of Rust. This also includes using new | |||
| features in a new release of Cargo, and requiring the use of a nightly-only | |||
| feature in a crate that previously worked on stable. | |||
|
|
|||
| Some projects choose to allow this in a minor release for various reasons. It | |||
| It is generally recommended to treat this as a minor change, rather than as | |||
There was a problem hiding this comment.
Would people agree with saying "strongly recommended"? The disadvantages are really significant and the advantage is minuscule.
There was a problem hiding this comment.
My reason for being conservative with this was I want this to be merged quickly under the idea that "some improvement is better than no improvement" and I want to avoid discussing too much "how strongly should we state this?". I would prefer we leave that to a follow up PR. Maybe no discussion is needed and thats great! I also know it can be easy for something like to have heavy discussion.
There was a problem hiding this comment.
Tbh this wording makes this read that there is a policy, and in fact against treating MSRV upgrades as breaking changes, ever.
It should be less strong e.g.
Most crates are careful with upgrading their minimum supported Rust version in patch releases, but will do it eventually. Usually then, an upgrade happens to a release that is some time in the past so that the breakage only affects a subset of users.
This is at least what I've seen most crates do in practice. It's only a minority of crates that immediately make a minor release using some new features, two days after release day and push it to users in a patch upgrade. There is just a lot of talk around that minority because they break builds for everyone else.
As for the link, IMO that discussion is a good start for reading about several pros and cons but it's still not something where there is consensus, so I'm not sure it's good to link it.
There was a problem hiding this comment.
Tbh this wording makes this read that there is a policy, and in fact against treating MSRV upgrades as breaking changes, ever.
Because there isn't a policy is why I still left this as a "possible-breaking". As we still leave room open, I am ok with this tentative default stance. We'll see what others on the cargo team think.
It should be less strong e.g.
That isn't just less strong but also adds in a lot of other information to be hashed out and I'm wanting to keep the scope narrow (calling out "patch releases").
As for having larger windows of MSRV support, that is covered later in this section.
As for the link, IMO that discussion is a good start for reading about several pros and cons but it's still not something where there is consensus, so I'm not sure it's good to link it.
- The link is specifically for the pros/cons
- Even then, it is the more definitive document at this time for a decision, so if we were to link to something for now, it would be that. I also feel it would give people the appropriate breadcrumb for where to go next, even if its not "decided" yet.
There was a problem hiding this comment.
This is about minor vs major, not minor vs patch
There was a problem hiding this comment.
but also adds in a lot of other information to be hashed out and I'm wanting to keep the scope narrow (calling out "patch releases").
I am ok with both "minor release" or "patch release", whichever is clearer.
If you write "it is generally recommended" then that sounds a lot like there is a policy.
I agree that "Some projects choose to allow this in a minor release for various reasons." should be replaced by something that is actually lived by the community. But the new wording definitely does not help either.
As for having larger windows of MSRV support, that is covered later in this section.
Actually that's a good point, maybe it might be better to edit it there. This statement makes it seem that most projects don't care about anything but the latest compiler, where only "some" projects support N releases back:
Rust also has a rapid 6-week release cycle, and some projects will provide compatibility
within a window of releases (such as the current stable release plus N
previous releases).
There was a problem hiding this comment.
The link is specifically for the pros/cons
I'm not a native speaker but from my impression, "for various reasons" has a bit of a double meaning. The diversity that "various" encodes can be interpreted in multiple ways. You can either read it that it's indeed a list of pros and cons, and I think this is what you intended. But you can also read it in the way that from multiple angles, it looks like that making waiting for the next MSRV increase until the next major release is a bad idea. I think it can often be misinterpreted as the second.
There was a problem hiding this comment.
I agree that "Some projects choose to allow this in a minor release for various reasons." should be replaced by something that is actually lived by the community. But the new wording definitely does not help either.
Wordsmithing is hard. If Instead say "most projects", I feel I'd need to verify that first
Actually that's a good point, maybe it might be better to edit it there. This statement makes it seem that most projects don't care about anything but the latest compiler, where only "some" projects support N releases back:
Do you feel that this PR has to expand scope to cover other improvements like this or can we leave that to future PRs?
There was a problem hiding this comment.
If Instead say "most projects", I feel I'd need to verify that first
To me it seems like a majority of crates do that, but it's just an impression, and doesn't base on hard data.
Do you feel that this PR has to expand scope to cover other improvements like this or can we leave that to future PRs?
I think one can do it in a future PR.
|
Also the text should probably say patch version may be bumped if the API didn't change which is compliant with semver spec. Bumping minor if no new functionality was added would go against the spec. (Although some prominent crates ignore it anyway.) |
Its not quite clear to me what you are referring to. Are you saying this generally in the document (which I would counter is out of scope for this PR) or specifically for MSRV changes? If for MSRV changes,
|
|
LGTM |
|
This looks good to me! I do expect this to evolve more when the api-guidelines moves forward, and hopefully something like #9930 will further make it possible to have clearer guidance. I'm going to do an fcp poll since this is a (albeit minor) policy change of making this recommended. @rfcbot fcp merge |
|
Team member @ehuss has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
proposed-final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
Thanks! @bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
Update cargo 8 commits in 13413c64ff88dd6c2824e9eb9374fc5f10895d28..09276c703a473ab33daaeb94917232e80eefd628 2023-05-10 13:46:18 +0000 to 2023-05-16 21:43:35 +0000 - docs: Clarify that crates.io doesn't link to docs.rs right away. (rust-lang/cargo#12146) - docs(ref): Clarify MSRV is generally minor (rust-lang/cargo#12122) - Fix `check_for_file_and_add`'s check for conflict file (rust-lang/cargo#12135) - Fixes: Incorrect document link (rust-lang/cargo#12143) - doc: intra-doc links and doc comments for build script (rust-lang/cargo#12133) - Add Cargo team charter. (rust-lang/cargo#12010) - Remove useless drop of copy type (rust-lang/cargo#12136) - Fix dep/feat syntax with hidden implicit optional dependencies (rust-lang/cargo#12130) r? ghost
rfcbot
added
finished-final-comment-period
For the second time in the last week or two, I've seen the wording of cargo's semver documentation used to justify pushing back against treating MSRV bumps as minor changes.
See time-rs/time#535 (comment). My hope is that this change will reduce confusion from readers which I hope will reduce conversation fatigue on this topic from maintainers.
The current wording makes it sound like "major" is the default stance but the general consensus seems to be around
"minor". I held back from changing from "possibly major" to minor" for now as the above linked policy isn't official yet and it leaves it open for crates to treat it as major.
I tried to keep the scope of this change small to reduce risk of the discussion being slowed down for specific points unrelated to my main goal from delaying the improvement I'm trying to make, the idea that "some improvement is better than no improvement".
Fixes #12121